-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: add automatic mode switch and command execution on task completion #8936
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add onComplete field to ModeConfig type with switchToMode, runCommand, and includeSummary options - Update attemptCompletionTool to check for and execute onComplete actions after task completion - Add comprehensive tests for automatic mode switching and command execution - Allow modes to define actions that should be taken automatically when a task completes Fixes #8935
Review SummaryI've reviewed the changes and identified 3 issues that need to be addressed: Issues Found
Key Concerns
|
|
|
||
| // Execute mode switch if specified | ||
| if (switchToMode) { | ||
| await cline.say("text", `Automatically switching to ${switchToMode} mode as configured...`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user-facing messages for automatic actions (e.g. 'Automatically switching to ... mode as configured...' and 'Automatically executing command: ...') are hardcoded. Consider using an internationalization function (like t()) to support localization of these messages.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| const provider = cline.providerRef.deref() | ||
| if (provider) { | ||
| const state = await provider.getState() | ||
| const currentMode = state?.mode || "code" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation uses the current provider mode (state?.mode) instead of the task's mode. This is incorrect because a task stores its own mode when created, and the onComplete configuration should be based on the mode the task was running in, not the current provider mode (which could be different if the user manually switched modes). Change to: const currentMode = await cline.getTaskMode() to use the task's actual mode.
| it("should execute mode switch when onComplete.switchToMode is configured", async () => { | ||
| const block: AttemptCompletionToolUse = { | ||
| type: "tool_use", | ||
| name: "attempt_completion", | ||
| params: { result: "Task completed successfully" }, | ||
| partial: false, | ||
| } | ||
|
|
||
| // Configure mode with onComplete.switchToMode | ||
| vi.mocked(getModeConfig).mockReturnValue({ | ||
| slug: "architect", | ||
| name: "Architect", | ||
| roleDefinition: "Test role", | ||
| groups: ["read", "edit"], | ||
| onComplete: { | ||
| switchToMode: "code", | ||
| }, | ||
| } as ModeConfig) | ||
|
|
||
| await attemptCompletionTool( | ||
| mockTask as Task, | ||
| block, | ||
| mockAskApproval, | ||
| mockHandleError, | ||
| mockPushToolResult, | ||
| mockRemoveClosingTag, | ||
| mockToolDescription, | ||
| mockAskFinishSubTaskApproval, | ||
| ) | ||
|
|
||
| // Verify mode switch was called | ||
| expect(mockHandleModeSwitch).toHaveBeenCalledWith("code") | ||
| expect(mockSay).toHaveBeenCalledWith("text", "Automatically switching to code mode as configured...") | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests mock getState() to return mode: "code" but then use getModeConfig to return different mode configurations. This creates an inconsistency that masks the bug in line 103 where the implementation uses state?.mode instead of cline.getTaskMode(). The tests should verify that getModeConfig is called with the task's mode, not the provider's current mode. Consider adding: expect(getModeConfig).toHaveBeenCalledWith(await mockTask.getTaskMode(), customModes)
| // Check for onComplete actions in the current mode configuration | ||
| const provider = cline.providerRef.deref() | ||
| if (provider) { | ||
| const state = await provider.getState() | ||
| const currentMode = state?.mode || "code" | ||
| const customModes = state?.customModes || [] | ||
|
|
||
| try { | ||
| const modeConfig = getModeConfig(currentMode, customModes) | ||
|
|
||
| if (modeConfig.onComplete) { | ||
| const { switchToMode, runCommand, includeSummary } = modeConfig.onComplete | ||
|
|
||
| // Prepare context for onComplete actions | ||
| const context = includeSummary ? result : undefined | ||
|
|
||
| // Execute mode switch if specified | ||
| if (switchToMode) { | ||
| await cline.say("text", `Automatically switching to ${switchToMode} mode as configured...`) | ||
| await provider.handleModeSwitch(switchToMode) | ||
| } | ||
|
|
||
| // Execute command if specified | ||
| if (runCommand) { | ||
| await cline.say("text", `Automatically executing command: ${runCommand}`) | ||
|
|
||
| // Create a new message with the command, optionally including the summary | ||
| let commandMessage = runCommand | ||
| if (context) { | ||
| commandMessage = `${runCommand}\n\nContext from previous task:\n${context}` | ||
| } | ||
|
|
||
| // Send the command as a new user message | ||
| await provider.postMessageToWebview({ | ||
| type: "invoke", | ||
| invoke: "sendMessage", | ||
| text: commandMessage, | ||
| }) | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // Log error but don't fail the completion | ||
| console.error("Failed to execute onComplete actions:", error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onComplete actions execute before checking if this is a subtask completion (which happens at line 145). This means subtasks will execute their mode's onComplete actions, potentially interfering with the parent task's flow. For example, if a subtask in Code mode has onComplete.switchToMode: "architect", it would switch modes instead of returning control to the parent task. The onComplete logic should be guarded with if (!cline.parentTask) to only execute for top-level tasks, not subtasks.
This PR attempts to address Issue #8935 by adding automatic mode switching and command execution after task completion.
Summary
This enhancement adds the ability for modes to define automatic actions that should occur when a task completes. Users can configure modes to automatically:
Implementation
Changes Made
Added
onCompletefield to ModeConfig type (packages/types/src/mode.ts)switchToMode: Optional mode to switch to after completionrunCommand: Optional command to execute after completionincludeSummary: Whether to include task result as context for the commandUpdated attemptCompletionTool (
src/core/tools/attemptCompletionTool.ts)onCompleteconfiguration in current modeAdded comprehensive tests (
src/core/tools/__tests__/attemptCompletionTool.spec.ts)Use Cases
This feature enables several workflow improvements:
Testing
All new functionality is covered by unit tests. The implementation gracefully handles errors and does not break task completion if onComplete actions fail.
Fixes #8935
Feedback and guidance are welcome!
Important
Adds automatic mode switching and command execution on task completion with configurable
onCompleteactions inModeConfig.onCompletefield toModeConfiginmode.tswithswitchToMode,runCommand, andincludeSummaryoptions.attemptCompletionToolinattemptCompletionTool.tsto executeonCompleteactions: mode switch, command execution, and optional summary inclusion.attemptCompletionTool.spec.tsfor mode switching, command execution, summary inclusion, and error handling.onCompleteconfigurations and provider availability.This description was created by
for 16f2f3f. You can customize this summary. It will automatically update as commits are pushed.